feat(checks): Improve GitHub output and check summary#38
Conversation
Only post PR reviews with inline comments, skip standalone summary comments that add noise without providing actionable inline feedback. The GitHub check annotations already provide the summary information. Fixes GH-36 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Add timing to runSkill so the duration is included in the report. This fixes the '-' appearing in the Duration column of the Skills table in the GitHub check summary. Fixes GH-36 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Redesign the core warden check summary to be more useful: - Lead with actionable headline showing issue count by severity - Display top 5 findings directly with title, location, description - Move skills table into collapsible details section - Use readable stats footer with labeled inline format The new design answers "what should I do?" instead of just "what happened?" Fixes GH-36 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
dcramer
changed the title
- Remove emojis from all output - Show findings with collapsible details (title visible, description hidden) - Remove useless "X findings across Y skills" header - Simplify check titles to just "X issues" or "No issues" - Show actual findings in skill check summary, not just severity table - Remove redundant Result column from skills table Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| lines.push(`*...and ${remaining} more*`, ''); | ||
| } | ||
| } else { | ||
| lines.push('No issues found.', ''); |
There was a problem hiding this comment.
Missing actionable headline in core check summary
Medium Severity
The buildCoreSummary function doesn't add the actionable headline described in the PR. The PR documentation states the summary leads with "🔴 X issues require attention" or "✅ No issues found", but the implementation starts directly with severity sections (### High) or just "No issues found." without the expected headline with emoji and issue count.
| } | ||
|
|
||
| /** Maximum findings to show in the summary */ | ||
| const MAX_SUMMARY_FINDINGS = 10; |
There was a problem hiding this comment.
Summary shows 10 findings instead of documented 5
Low Severity
The MAX_SUMMARY_FINDINGS constant is set to 10, but the PR description explicitly states the summary "Shows top 5 findings directly with title, location, and description." This mismatch means the implementation displays twice as many findings as the documented design intended, potentially reducing the actionability the redesign aims to achieve.
|
|
||
| // Add aggregate stats line if available | ||
| const hasStats = data.totalDurationMs !== undefined || data.totalUsage; | ||
| if (hasStats) { |
There was a problem hiding this comment.
Duplicate formatLocation function reimplements existing utility
Low Severity
The new formatLocation function duplicates the core logic of the existing formatLocation in src/cli/output/formatters.ts. Both format path:startLine-endLine the same way. The only difference is the new version takes an object parameter and wraps output in backticks. This could reuse the existing utility by calling it and wrapping the result, rather than reimplementing the line-range formatting logic.
| lines.push(finding.description, ''); | ||
| lines.push('</details>', ''); | ||
| } | ||
| } |
There was a problem hiding this comment.
Findings-by-severity rendering logic duplicated across functions
Low Severity
The logic for sorting findings by severity, grouping them into a Map, iterating over severityOrder, capitalizing labels, and rendering each finding with <details> tags is nearly identical in both buildSkillSummary and buildCoreSummary. This ~25-line block is duplicated, including the same hardcoded severityOrder array. A helper function to render findings grouped by severity would eliminate this duplication.
Additional Locations (1)
1 participant
Improves GitHub output based on feedback in #36.
Changes
1. Remove standalone PR summary comments
Only post PR reviews with inline comments - skip standalone summary comments
that add noise without providing actionable inline feedback.
2. Track skill duration
Add
durationMstracking torunSkill()so the duration column shows values.3. Redesign check summaries for actionability
Example output
Fixes #36